change and fixes for existing funcs #6
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
-- default device is always available.
Solution is not to create context on devices that are not
of the requested device type or not from the requested backend.
```
Python 3.7.7 (default, Jul 14 2020, 22:02:37)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.17.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: import dpctl
In [2]: with dpctl.device_context('opencl:gpu'):
...: print(1)
...:
1
In [3]: with dpctl.device_context('level0:gpu'):
...: print(1)
...:
1
```
…rupd/dpctl into feature/backend_specific_queues_v2
* CC=clang.exe * CC=clang-cl.exe
* CC=clang.exe * CC=clang-cl.exe
…ific_queues_v2 Feature/backend specific queues v2
* Add support for uint8 and ulong.
* Rename all functions to addressof_ref and add docstring. Co-authored-by: reazul.hoque <[email protected]>
* Rename readme and headers * Fix left over places where DPPL/PyDPPL was used in comments. Co-authored-by: Diptorup Deb <[email protected]>
* Fix unhandled character * Fix unhandled character * Remove unused imports * Fix class name duplication
Warning about conda-build=3.20
* Add changelog Co-authored-by: etotmeni <[email protected]> Co-authored-by: Diptorup Deb <[email protected]>
This ensures that backend libraries are copied into dpctl and included in the conda tar ball. Also fixed copy statement to account for .lib files being in %INSTALL_PREFIX%\lib and .dll files being in %INSTALL_PREFIX%\bin on Windows.
Use INSTALL_PATH in cmake call
* Fixed typo in the code for set_default_queue Allowing both strings or enums as selectors for backend and device. * Fixed "variabel reference before assignment" in device_context manager ctxt must be set outside of try/finally to be guaranteed avaialble in the finally clause.
diptorupd
reviewed
Oct 13, 2020
dpctl/sycl_core.pyx
Outdated
| max_work_item_sizes = [] | ||
| for n in range(3): | ||
| max_work_item_sizes.append(self._max_work_item_sizes[n]) | ||
| DPPLSize_t_Array_Delete(self._max_work_item_sizes) |
Owner
There was a problem hiding this comment.
Should the DPPLSize_t_Array_Delete call be here? I think should be moved to the __dealloc__ otherwise the self._max_work_item_sizes is deleted after first call to get_max_work_item_sizes.
|
|
||
| def test_get_max_compute_units (self): | ||
| q = dpctl.get_current_queue() | ||
| try: |
Owner
There was a problem hiding this comment.
Also test if max_compute_units is returned some expected value and not garbage?
dpctl/tests/test_sycl_device.py
Outdated
| class TestSyclDevice (unittest.TestCase): | ||
|
|
||
| def test_get_max_compute_units (self): | ||
| q = dpctl.get_current_queue() |
Owner
There was a problem hiding this comment.
Maybe check for exception on get_current_queue as well since that call can raise a SyclQueueCreationError.
| def test_get_max_work_item_dims (self): | ||
| q = dpctl.get_current_queue() | ||
| try: | ||
| max_work_item_dims = q.get_sycl_device().get_max_work_item_dims() |
| def test_get_max_work_item_sizes (self): | ||
| q = dpctl.get_current_queue() | ||
| try: | ||
| max_work_item_sizes = q.get_sycl_device().get_max_work_item_sizes() |
| def test_get_max_work_group_size (self): | ||
| q = dpctl.get_current_queue() | ||
| try: | ||
| max_work_group_size = q.get_sycl_device().get_max_work_group_size() |
| def test_get_max_num_sub_groups (self): | ||
| q = dpctl.get_current_queue() | ||
| try: | ||
| max_num_sub_groups = q.get_sycl_device().get_max_num_sub_groups() |
diptorupd
reviewed
Oct 13, 2020
| */ | ||
| DPPL_API | ||
| bool | ||
| DPPLDevice_GetAspectsBaseAtomics (__dppl_keep const DPPLSyclDeviceRef DRef); |
Owner
There was a problem hiding this comment.
Let us rename to DPPLDevice_HasInt64BaseAtomics.
| */ | ||
| DPPL_API | ||
| bool | ||
| DPPLDevice_GetAspectsExtendedAtomics (__dppl_keep const DPPLSyclDeviceRef DRef); |
Owner
There was a problem hiding this comment.
let us rename to DPPLDevice_HasInt64ExtendedAtomics
| { | ||
| auto D = unwrap(DRef); | ||
| if(D) { | ||
| return D->has(aspect::int64_base_atomics); |
Owner
There was a problem hiding this comment.
I did not know about the functions. Nice!
| if(!OpenCL_cpu) | ||
| GTEST_SKIP_("Skipping as no OpenCL CPU device found."); | ||
|
|
||
| auto n = DPPLDevice_GetAspectsBaseAtomics(OpenCL_cpu); |
Owner
There was a problem hiding this comment.
What do we expect on devices that actually do not have int64_base_atomics? In such cases false is the right result. I think a better way to test this is to include CL::sycl.hpp and call the get the aspect for the device using SYCL and see if it matches what DPPLDevice_xxx returns. It is not really a great test, just sanity check.
|
|
||
| auto n = DPPLDevice_GetAspectsBaseAtomics(OpenCL_gpu); | ||
| EXPECT_TRUE(n != false); | ||
| } |
| GTEST_SKIP_("Skipping as no OpenCL CPU device found."); | ||
|
|
||
| auto n = DPPLDevice_GetAspectsExtendedAtomics(OpenCL_cpu); | ||
| EXPECT_TRUE(n != false); |
| GTEST_SKIP_("Skipping as no OpenCL GPU device found."); | ||
|
|
||
| auto n = DPPLDevice_GetAspectsExtendedAtomics(OpenCL_gpu); | ||
| EXPECT_TRUE(n != false); |
|
|
||
| def test_get_aspects_base_atomics (self): | ||
| q = dpctl.get_current_queue() | ||
| try: |
Owner
There was a problem hiding this comment.
Aha now I see. Basically we are testing that the function should never raise an exception. OK.
1e-to
force-pushed
the
extra_device_info
branch
from
75a229a to
e3c901d
Compare
This GitHub Action checks that code in pull request is formatted with black. See https://black.readthedocs.io/en/stable/github_actions.html
versioneer.py is not a part of the project. Add pyproject.toml file with section [tool.black] and parameter exclude = 'versioneer.py'. This file will be used by developers and GitHub Actions for black. Usign pyproject.toml allows use black like this: `black .`
Use black version 20.8b1 or branch [stable](https://github.com/psf/black/tree/stable)). Command: black .
File .git-blame-ignore-revs contains a list of revisions to use for ignoring in blame. Example: $ git blame FILE --ignore-revs-file .git-blame-ignore-revs Also you can configure git to automatically ignore revisions listed in a file on every call to git blame: $ git config blame.ignoreRevsFile .git-blame-ignore-revs Requirements: Git >=2.23
Add black formatter for Python code
…t-queue-on-C-side dpctl.get_current_queue is now cimportable
| DPPL_API | ||
| uint32_t | ||
| DPPLDevice_GetMaxComputeUnites (__dppl_keep const DPPLSyclDeviceRef DRef); | ||
| DPPLDevice_GetMaxComputeUnits (__dppl_keep const DPPLSyclDeviceRef DRef); |
There was a problem hiding this comment.
It is very dangerous to rename API function. It is better to create a new one with new name and implement the old one via the new one.
There was a problem hiding this comment.
Then mark it as deprecated. Better to print warning in stderr when old function is used.
And remove in the next release.
* Del old ocl interface * Del cffi * Fix CMakeLists * Remove OpenCL from README.md * Update changelog Co-authored-by: etotmeni <[email protected]> Co-authored-by: Sergey Pokhodenko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For now in this PR I did following:
DPPLDevice_GetMaxComputeUnitestoDPPLDevice_GetMaxComputeUnitsWIP